Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Props table #256

Merged
merged 15 commits into from
May 9, 2024
Merged

Props table #256

merged 15 commits into from
May 9, 2024

Conversation

franckgaudin
Copy link

@franckgaudin franckgaudin commented May 7, 2024

this pull request focuses on the integration of a component's property table.
The choice to use @tanstack/react-table was made because React Aria Component wasn't displaying the data correctly. An investigation was carried out without finding a convincing solution, which is why I chose to use tanstack. What's more, it can easily be replaced by another solution in the future.

The next steps will be navigation between the components (menu on the right) and integration of the table of content.

Page: https://deploy-preview-256--wl-hopper.netlify.app/components/button

Copy link

changeset-bot bot commented May 7, 2024

⚠️ No Changeset found

Latest commit: a6f3aba

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

netlify bot commented May 7, 2024

Deploy Preview for wl-hopper ready!

Name Link
🔨 Latest commit a6f3aba
🔍 Latest deploy log https://app.netlify.com/sites/wl-hopper/deploys/663d09ae74f8930008371367
😎 Deploy Preview https://deploy-preview-256--wl-hopper.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Franck Gaudin added 2 commits May 7, 2024 15:39
…ps-table

# Conflicts:
#	apps/docs/app/pages/landing/page.tsx
#	apps/docs/package.json
#	pnpm-lock.yaml
Copy link
Contributor

github-actions bot commented May 7, 2024

View Storybook

@franckgaudin franckgaudin requested a review from vicky-comeau May 9, 2024 12:22
alexasselin008
alexasselin008 previously approved these changes May 9, 2024
@fraincs
Copy link
Contributor

fraincs commented May 9, 2024

image
I see a mix of single and double quotes in the default column

@fraincs
Copy link
Contributor

fraincs commented May 9, 2024

image

Is there a reason why we moved the # inside the title? This feels weird when in a Disclosure

@fraincs
Copy link
Contributor

fraincs commented May 9, 2024

image
Could we merge both statements?

@fraincs
Copy link
Contributor

fraincs commented May 9, 2024

image
When the website and the samples are in dark mode, the moon logo is not visible.

@franckgaudin
Copy link
Author

franckgaudin commented May 9, 2024

I see a mix of single and double quotes in the default column

It seems that the single cote come from React Aria components, I'll see if there is a way to reformat them .

@franckgaudin
Copy link
Author

Is there a reason why we moved the # inside the title? This feels weird when in a Disclosure

It has been moved so that it remains inside the collapse. We could replace the # sign with the link icon to make it clearer.

Looking at other DS, I realized that this position is a matter of preference.

Example:

Polaris

polaris-link

React Aria Component

Capture d’écran, le 2024-05-09 à 12 02 58

Atlassian

Capture d’écran, le 2024-05-09 à 12 03 51

alexasselin008
alexasselin008 previously approved these changes May 9, 2024
@franckgaudin franckgaudin merged commit a2480a3 into main May 9, 2024
8 checks passed
@franckgaudin franckgaudin deleted the feature/hop-20_integration-of-props-table branch May 9, 2024 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants